Skip to content

Add back onTelemetry handler for desktop - #19254

Merged
Karthik Nadig (karthiknadig) merged 1 commit into
microsoft:mainfrom
debonte:fixPylanceTelemetry
Jun 8, 2022
Merged

Add back onTelemetry handler for desktop#19254
Karthik Nadig (karthiknadig) merged 1 commit into
microsoft:mainfrom
debonte:fixPylanceTelemetry

Conversation

@debonte

@debonte Erik De Bonte (debonte) commented Jun 8, 2022

Copy link
Copy Markdown

Fixes https://github.com/microsoft/pyrx/issues/2480

The LanguageClient.onTelemetry handler that relays telemetry from Pylance was accidentally removed for desktop scenarios in #18884. So all ms-python.python/language_server/* events are missing for desktop users on Python 2022.5.11091004 and later.

Telemetry still worked for vscode.dev because we still had an onTelemetry handler for browser.

This PR adds back the removed desktop handler.

I'd love to add a test for this, but didn't see any existing tests that deal with LanguageClient or NodeLanguageServerProxy. Added #19261 to track adding a test for this scenario.

@karthiknadig

Copy link
Copy Markdown
Member

There is no framework to test this right now. We will have to add a new one. For now, I recommend adding work item to create tests for this.

@karthiknadig

Copy link
Copy Markdown
Member

Work item for test: #19261

@karthiknadig
Karthik Nadig (karthiknadig) merged commit aed7da9 into microsoft:main Jun 8, 2022
@debonte
Erik De Bonte (debonte) deleted the fixPylanceTelemetry branch June 8, 2022 20:20
Karthik Nadig (karthiknadig) pushed a commit to karthiknadig/vscode-python that referenced this pull request Jun 8, 2022
Add back LanguageClient.onTelemetry call for desktop
Karthik Nadig (karthiknadig) pushed a commit that referenced this pull request Jun 8, 2022
Add back LanguageClient.onTelemetry call for desktop
Wes McKinney (wesm) pushed a commit to posit-dev/positron that referenced this pull request Mar 28, 2024
Add back LanguageClient.onTelemetry call for desktop
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog No news entry required skip tests Updates to tests unnecessary

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants